Repository navigation
Check the buffer size of orthogonal selections - #787
Open
FabianIsensee wants to merge 1 commit into
Open
FabianIsensee wants to merge 1 commit into
FabianIsensee wants to merge 1 commit into
Conversation
get_oindex_numpy() and set_oindex_numpy() pass typesize * prod(selection lengths) to the C orthogonal selection as the buffer size, but their callers allocate the NumPy buffer for the array's dtype. When the dtype's items are smaller than the typesize, the C copy writes (get) or reads (set) past the end of that buffer. This happens with valid files: c-blosc2 2.7 to 2.16 stored "|u1" as the dtype of arrays created without one, whatever the typesize, e.g. c-blosc2's own plugins/test_data/example_item_prices.b2nd (float32 items, typesize 4), whose get_oselection_numpy() segfaults. A damaged dtype string does the same. oindex, take(axis=...) and integer-array indexing of multi-dimensional arrays go through the same two functions. Check the length of the buffer against the selection at the start of both functions, before the index arrays are allocated, and raise ValueError when it is too small, as get_sparse_numpy() does (slices are checked by c-blosc2, which gets their real buffer length). This does not depend on the buffer check of the C orthogonal selection, which accepted too small buffers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DiPFzu8ziFQsxrJUkHCrzx
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
get_oindex_numpy()andset_oindex_numpy()passtypesize * prod(selection lengths)tob2nd_get/set_orthogonal_selection()as the buffer size, while their callers allocate the NumPy buffer for the array's dtype. When the dtype's items are smaller than the typesize, c-blosc2 writes (get) or reads (set) past the end of that buffer.That happens with valid files. c-blosc2 2.7 to 2.16 stored
"|u1"as the dtype of arrays created without one, whatever the typesize (2.17 writes"|S{typesize}"), for example c-blosc2's ownplugins/test_data/example_item_prices.b2nd(float32 items, typesize 4), which python-blosc2 opens as uint8:blosc2.open("example_item_prices.b2nd").get_oselection_numpy([range(12), range(25), range(250)])segfaults (4.14.1 and main). Under ASan: heap-buffer-overflow WRITE incopy_block_buffer_data()right after the 75000-byte result buffer (the selection is 300000 bytes).oindex[...]get and set,take(..., axis=...)and integer-array indexing of n-D arrays (a[[0, 1]], throughtake(axis=0)) go through the same two functions and do the same. A one-item selection writes 4 bytes into a 1-byte buffer without crashing; a set reads past the value and writes those bytes into the file.<f8became<f2) does the same.The other read paths already refuse these arrays:
a[...], slices,np.asarray(a)raiseRuntimeError: Error while getting the buffer(c-blosc2 gets the real buffer length there), and flattake()raises inget_sparse_numpy(), which checksview.len.This PR checks the buffer's length against the selection at the start of both functions, before the index arrays are allocated, and raises
ValueError: destination buffer is smaller than the requested orthogonal selection (75000 < 300000 bytes): its itemsize is 1, but the array's typesize is 4("source" for writes). Larger buffers are accepted as before. The size passed to c-blosc2 is unchanged, so this works with and without Blosc/c-blosc2#864 (which fixes the inverted buffer check inorthogonal_selection()). The check sits outside the lines that #770 restructures, so the two PRs merge in either order; it also merges cleanly with #779.Tests, in the new
tests/ndarray/test_oindex_typesize.py:example_day_month_temp.b2ndfrom c-blosc2'splugins/test_data(2.6 KB, copied intotests/data/legacy_u1_dtype/) read and written through oselection,oindexanda[[0, 1]]; the layout ofexample_item_prices.b2nd(141 KB, not copied) and a float64 array with the dtype string patched to"|u1"in a cframe; a damaged dtype string (<f8-><f2); too small, exact, larger and empty buffers passed to the extension methods directly; and valid arrays of several dtypes (bool, big-endian int32,S7,<U100with 400-byte items, structured) throughoindex. The legacy, damaged and buffer tests crash or fail on main; all pass with the fix. Full suite: same results as main (only the 2 b2view tests that need aiohttp fail locally). ASan build: no reports in these tests, the indexing test files, and 37 read/write calls on the example file.Cost: about 0.05-0.15 µs per call, for a second buffer acquisition and the size product (in Python ints, which cannot wrap around): ~2-3 µs one-block selections get 2-5% slower, a 9 µs one 0.5-1.6%, take and writes do not change measurably (Ryzen 9 7950X, 1 thread, two runs of 6 alternated rounds, main built twice for the noise floor: within 1.1%). After #770 the check could move behind its
PyObject_GetBufferand comparebuf.lenwithbuffersize_for free, and once the bundled c-blosc2 has #864, passingbuf.lenwould let c-blosc2 do the check; I kept it separate here so the PRs do not depend on each other.Not in this PR: these legacy arrays still cannot be read through NumPy (as before, every read path now raises instead of crashing;
tobytes()works). If you want them to read, python-blosc2 could present a stored"|u1"with a typesize above 1 asS{typesize}, as c-blosc2 >= 2.17 writes such arrays and as python-blosc2 already shows arrays without a dtype. That changes.dtypefor these files, so I left it out; happy to do it in a follow-up if you want it.I used Claude Code (Anthropic) to trace the bug, write the fix and the tests, and run the measurements; I reviewed the change and the numbers.
🤖 Generated with Claude Code